Skip to content

276 handle subassignemnts with the same object #277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Oct 31, 2025
Merged

276 handle subassignemnts with the same object #277

merged 10 commits into from
Oct 31, 2025

Conversation

@m7pr
Copy link
Contributor

@m7pr m7pr commented Oct 31, 2025 •
edited
Loading

Fixes #276

> teal.data::teal_data() |> within({ 
    ADSL <- teal.data::rADSL
    ADSL$Yada[ADSL$AGE > 30] <- 1
 }) |> teal.code::get_code(names = "ADSL") |> cat()

now gives the second line

ADSL <- teal.data::rADSL
ADSL$Yada[ADSL$AGE > 30] <- 1

@m7pr m7pr added the core label Oct 31, 2025
@m7pr m7pr mentioned this pull request Oct 31, 2025
Closed
3 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025 •
edited
Loading

Unit Tests Summary

  1 files   14 suites   6s ⏱️
183 tests 180 ✅ 3 💤 0 ❌
267 runs  264 ✅ 3 💤 0 ❌

Results for commit 3a6ece3.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025 •
edited
Loading

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
utils-get_code_dependency 👶 $+0.05$ get_code_with_subassignments_handles_complex_nested_subassignments
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_handles_subassignments_with_data_frame_column_creation
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_handles_subassignments_with_logical_indexing
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_handles_subassignments_with_matrix_indexing
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_handles_subassignments_with_multiple_operators
utils-get_code_dependency 👶 $+0.03$ get_code_with_subassignments_tracks_attributes_function_with_subassignments
utils-get_code_dependency 👶 $+0.05$ get_code_with_subassignments_tracks_multiple_subassignments_to_same_object
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_tracks_nested_subassignments
utils-get_code_dependency 👶 $+0.04$ get_code_with_subassignments_tracks_operator_with_subassignments
utils-get_code_dependency 👶 $+0.07$ get_code_with_subassignments_tracks_subassignment_as_producing_the_base_object
utils-get_code_dependency 👶 $+0.09$ get_code_with_subassignments_tracks_subassignments_with_complex_expressions
utils-get_code_dependency 👶 $+0.03$ get_code_with_subassignments_tracks_subassignments_with_function_calls_on_LHS

Results for commit d43ebd6

♻️ This comment has been updated with latest results.

@m7pr m7pr marked this pull request as draft October 31, 2025 09:19
m7pr
m7pr commented Oct 31, 2025
View reviewed changes
m7pr
m7pr commented Oct 31, 2025
View reviewed changes
m7pr
m7pr commented Oct 31, 2025
View reviewed changes
@m7pr m7pr marked this pull request as ready for review October 31, 2025 09:57
Copilot AI reviewed Oct 31, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the code dependency tracking functionality to properly handle subassignments in R (e.g., x[1] <- value, df$col <- data). The changes fix issues with duplicate handling in dependency extraction and add comprehensive test coverage for various subassignment patterns.

Key changes:

  • Improved duplicate handling when moving elements in parentheses to the right-hand side of dependency graphs
  • Fixed the move_functions_after_arrow function to preserve duplicates and handle edge cases properly
  • Added 13 new test cases covering various subassignment scenarios including bracket indexing, nested assignments, and complex expressions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
R/utils-get_code_dependency.R Refactored duplicate handling logic in extract_occurrence and move_functions_after_arrow functions to properly track subassignments
tests/testthat/test-utils-get_code_dependency.R Added comprehensive test suite for subassignment tracking with 13 test cases covering various patterns
DESCRIPTION Updated RoxygenNote version from 7.3.2 to 7.3.3

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025 •
edited
Loading

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------
R/qenv-c.R                          55       0  100.00%
R/qenv-class.R                      13       0  100.00%
R/qenv-concat.R                      7       0  100.00%
R/qenv-constructor.R                 1       0  100.00%
R/qenv-errors.R                      4       4  0.00%    6-9
R/qenv-eval_code.R                  60       1  98.33%   40
R/qenv-extract.R                    30       0  100.00%
R/qenv-get_code.R                   24       0  100.00%
R/qenv-get_env.R                     3       1  66.67%   27
R/qenv-get_messages.r                5       0  100.00%
R/qenv-get_outputs.R                 6       0  100.00%
R/qenv-get_var.R                    13       1  92.31%   13
R/qenv-get_warnings.R                5       0  100.00%
R/qenv-join.R                        1       1  0.00%    13
R/qenv-length.R                      2       1  50.00%   2
R/qenv-show.R                       29      29  0.00%    19-50
R/qenv-within.R                      8       0  100.00%
R/utils-get_code_dependency.R      265       3  98.87%   160, 258, 341
R/utils.R                           42       0  100.00%
TOTAL                              573      41  92.84%

Diff against main

Filename                         Stmts    Miss  Cover
-----------------------------  -------  ------  -------
R/utils-get_code_dependency.R      +11       0  +0.05%
TOTAL                              +11       0  +0.14%

Results for commit: 3a6ece3

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

m7pr and others added 2 commits October 31, 2025 11:22
@m7pr
Update R/utils-get_code_dependency.R
2a4a6cf 
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Marcin <[email protected]>
@m7pr
Update R/utils-get_code_dependency.R
7ca59f9 
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Marcin <[email protected]>
llrs-roche
llrs-roche approved these changes Oct 31, 2025
View reviewed changes
Copy link
Contributor

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great quick fix! The tests are long and cover many cases!

@llrs-roche llrs-roche self-assigned this Oct 31, 2025
averissimo
averissimo reviewed Oct 31, 2025
View reviewed changes
m7pr
m7pr commented Oct 31, 2025
View reviewed changes
@m7pr m7pr enabled auto-merge (squash) October 31, 2025 12:52
@m7pr m7pr merged commit a3460d5 into main Oct 31, 2025
28 checks passed
@m7pr m7pr deleted the 276_subassignemnets branch October 31, 2025 12:55
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@averissimo averissimo averissimo left review comments

Copilot code review Copilot Copilot left review comments

@llrs-roche llrs-roche llrs-roche approved these changes

Assignees

@llrs-roche llrs-roche

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Code on the teal_data is omitted

4 participants

@m7pr @averissimo @llrs-roche